Skip to content

Conversation

RKSimon
Copy link
Collaborator

@RKSimon RKSimon commented Aug 18, 2025

Avoid unnecessary (costly) computeKnownBits/ComputeNumSignBits calls - use MaskedValueIsZero instead of computeKnownBits directly to simplify code.

…its failures. NFC.

Avoid unnecessary (costly) computeKnownBits/ComputeNumSignBits calls - use MaskedValueIsZero instead of computeKnownBits directly to simplify code.
@RKSimon RKSimon requested a review from woruyu August 18, 2025 12:57
@llvmbot llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Aug 18, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 18, 2025

@llvm/pr-subscribers-llvm-selectiondag

Author: Simon Pilgrim (RKSimon)

Changes

Avoid unnecessary (costly) computeKnownBits/ComputeNumSignBits calls - use MaskedValueIsZero instead of computeKnownBits directly to simplify code.


Full diff: https://github.com/llvm/llvm-project/pull/154111.diff

1 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+6-9)
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 785245b2d9e74..0e63f051b1d65 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -16332,25 +16332,22 @@ SDValue DAGCombiner::visitTRUNCATE(SDNode *N) {
     // (trunc (abdu/abds a, b)) -> (abdu/abds (trunc a), (trunc b))
     if ((!LegalOperations || N0.hasOneUse()) &&
         TLI.isOperationLegal(N0.getOpcode(), VT)) {
-      EVT SrcVT = N0.getValueType();
       EVT TruncVT = VT;
       unsigned SrcBits = SrcVT.getScalarSizeInBits();
       unsigned TruncBits = TruncVT.getScalarSizeInBits();
-      unsigned NeededBits = SrcBits - TruncBits;
 
       SDValue A = N0.getOperand(0);
       SDValue B = N0.getOperand(1);
       bool CanFold = false;
 
       if (N0.getOpcode() == ISD::ABDU) {
-        KnownBits KnownA = DAG.computeKnownBits(A);
-        KnownBits KnownB = DAG.computeKnownBits(B);
-        CanFold = KnownA.countMinLeadingZeros() >= NeededBits &&
-                  KnownB.countMinLeadingZeros() >= NeededBits;
+        APInt UpperBits = APInt::getBitsSetFrom(SrcBits, TruncBits);
+        CanFold = DAG.MaskedValueIsZero(A, UpperBits) &&
+                  DAG.MaskedValueIsZero(B, UpperBits);
       } else {
-        unsigned SignBitsA = DAG.ComputeNumSignBits(A);
-        unsigned SignBitsB = DAG.ComputeNumSignBits(B);
-        CanFold = SignBitsA > NeededBits && SignBitsB > NeededBits;
+        unsigned NeededBits = SrcBits - TruncBits;
+        CanFold = DAG.ComputeNumSignBits(A) > NeededBits &&
+                  DAG.ComputeNumSignBits(B) > NeededBits;
       }
 
       if (CanFold) {

Comment on lines 16345 to 16346
CanFold = DAG.MaskedValueIsZero(A, UpperBits) &&
DAG.MaskedValueIsZero(B, UpperBits);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
CanFold = DAG.MaskedValueIsZero(A, UpperBits) &&
DAG.MaskedValueIsZero(B, UpperBits);
CanFold = DAG.MaskedValueIsZero(B, UpperBits) &&
DAG.MaskedValueIsZero(A, UpperBits);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why to check the second oprand for ABDU/ABDS firstly, in my understadning, the second is smaller, the possibility of MaskedValueIsZero will be bigger.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for commutable instructions we move constants to the RHS, in the middle-end the node is often canonicalized to the RHS being the simpler in other ways as well but we don't tend to bother as much in DAG.

Comment on lines 16349 to 16350
CanFold = DAG.ComputeNumSignBits(A) > NeededBits &&
DAG.ComputeNumSignBits(B) > NeededBits;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
CanFold = DAG.ComputeNumSignBits(A) > NeededBits &&
DAG.ComputeNumSignBits(B) > NeededBits;
CanFold = DAG.ComputeNumSignBits(B) > NeededBits &&
DAG.ComputeNumSignBits(A) > NeededBits;

Copy link
Member

@woruyu woruyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@RKSimon RKSimon merged commit 858d1df into llvm:main Aug 18, 2025
9 checks passed
@RKSimon RKSimon deleted the dag-trunc-abd-valuetracking branch August 18, 2025 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:SelectionDAG SelectionDAGISel as well
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants